Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication favorites #253

Open
wants to merge 75 commits into
base: authorization
Choose a base branch
from

Conversation

ayrtoncravero
Copy link
Contributor

@ayrtoncravero ayrtoncravero commented Jan 26, 2022

Autenticacion en el modulo favorites

Para completar la tarea se agrego lo siguiente:

  • favorite.controller:
    • Decorador: '@ApiBearerAuth'
    • Decorador: '@ApiForbiddenResponse'
    • Decorador: '@ApiUnauthorizedResponse'
    • Logica para validar si el usuario cuenta con la autorización pertinente dependiendo la acción
  • favorite.controller.spec:
    • Se agregaron los test para todos los casos de uso:
      image
  • create-favorite.dto: Se modifico para agregar lo campos necesarios para la creación
  • favorite.service:
    • Se agregaron modificaciones para su correcto funcionamiento.
  • favorite.service.spec:
    • Se agregaron los test para los casos de uso:
      image
  • favorites.module: se importo 'caslModule' en '@module'
  • casl-ability: Se agregaron cans para cada accion de Favorite

…umentation for 'forbidden' was added, a method that was not going to be used was removed and an import was removed
@ayrtoncravero ayrtoncravero requested a review from ghnoob January 28, 2022 15:18
Copy link
Contributor

@ghnoob ghnoob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisar las conversaciones que no estan resueltas

src/favorites/controllers/favorites.controller.spec.ts Outdated Show resolved Hide resolved
src/favorites/controllers/favorites.controller.spec.ts Outdated Show resolved Hide resolved
src/favorites/controllers/favorites.controller.spec.ts Outdated Show resolved Hide resolved
@ayrtoncravero ayrtoncravero requested a review from ghnoob January 29, 2022 00:31
Copy link
Contributor

@ghnoob ghnoob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tambien quedaron dos conversaciones sin resolver

Comment on lines +33 to 45
async create(body: CreateFavoriteDto, user: User): Promise<void> {
const itemExist = await this.itemRepo.findOne(body.itemId);

if (!itemExist) {
throw new UnprocessableEntityException('Item does not exist');
}

const preFavorite: any = { ...body, user: user };

const newFavorite = this.favoritesRepo.create(preFavorite);

await this.favoritesRepo.save(newFavorite);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • falta asignarle el item al favorito
  • hay que comprobar que no se pueda agregar el mismo favorito dos veces
  • cuando el item no existe esta tirando la excepcion bien pero no lo esta manejando nest por lo que devuelve 201 igual

name: 'id',
type: Number,
required: true,
description: 'The ID of the Favorite record to delete.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revisar description

Comment on lines +270 to +272
if (ability.cannot(Permission.Read, subject('Favorite', favorite))) {
throw new ForbiddenException();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

esta rebotando todas las peticiones, incluso cuando no tendria que hacerlo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autorización Favorites
2 participants